Skip to content

Conversation

@metcoder95
Copy link
Member

This relates to...

Closes #4230

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

@metcoder95 metcoder95 requested review from mcollina and ronag June 26, 2025 10:17
Comment on lines 173 to 175
if (err != null) {
// TODO: should we drop the session?
client[kOnError](err)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinkin' twice, I'm over the fence of we should fail rather than actually just emit an event

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should drop the sesson if this error.

* @description Time interval between PING frames dispatch
* @default 60000
*/
pingInterval?: number;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this type needs a test

@metcoder95 metcoder95 requested a review from mcollina July 4, 2025 08:04
Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@metcoder95
Copy link
Member Author

@mcollina

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina
Copy link
Member

@metcoder95 please resolve the conflict

@metcoder95
Copy link
Member Author

Done

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 86.53846% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.84%. Comparing base (b7f5fca) to head (5b8dcbb).

Files with missing lines Patch % Lines
lib/dispatcher/client-h2.js 82.50% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4296      +/-   ##
==========================================
- Coverage   92.85%   92.84%   -0.02%     
==========================================
  Files         106      106              
  Lines       33111    33157      +46     
==========================================
+ Hits        30745    30784      +39     
- Misses       2366     2373       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose HTTP/2 ping

5 participants